[mono-move] Chained field access fusion#20249
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
fceccc9 to
d55336f
Compare
d55336f to
bd474d4
Compare
mono-move benchmark gate1 regression(s) beyond ±3% noise band
|
| pub(crate) fn mut_local_borrow_target(instr: &Instr) -> Option<Slot> { | ||
| if let Instr::MutBorrowLoc(_, local) | ||
| | Instr::MutBorrowLocField(_, _, _, local) | ||
| | Instr::MutBorrowLocalFieldChain(_, _, local) = instr |
There was a problem hiding this comment.
It is safer to make this an exhaustive match?
| | Instr::WriteLocalField(owner, _, _, _) => Some((*owner, NominalKind::Struct)), | ||
|
|
||
| // A struct chain's first owner contains every later owner as an inline | ||
| // field, so discovering it transitively lays out the whole path. |
There was a problem hiding this comment.
This seems like a silent assumption, what if we have boxed struct later? Maybe we can have this as some stronger invariant?
| MicroOp::Move8 { dst, src } => { | ||
| let v = read_u64(fp, src); | ||
| write_u64(fp, dst, v); | ||
| // The slots need not be 8-aligned (see `Move8`'s doc), |
There was a problem hiding this comment.
We should re-audit similar instructions / APIs, can add a TODO(security)? Also I think HeapMoveFrom8, HeapMoveTo8, HeapMoveTo8Imm have same issue
| U256::from_le_bytes(bytes) | ||
| } | ||
|
|
||
| /// Shared prologue of the tag-dispatched variant-field ops |
There was a problem hiding this comment.
I think this documentation can be more concise focusing on what is returned, and invariants/ error conditions, the fact that is is some prologue is not very relevant
| // scratch. Non-overlapping: `dst` is a stack-region | ||
| // slot, the field is heap-object bytes. | ||
| let (obj_ptr, offset) = variant_field_loc(fp, enum_ref, offsets)?; | ||
| std::ptr::copy_nonoverlapping( |
There was a problem hiding this comment.
Here and below: let's have // Non-overlapping: reasoning in its own line so it is clearer and closer to actual call?
| ref offsets, | ||
| size, | ||
| } => { | ||
| // Fuses the tag-dispatched borrow and the read, no |
There was a problem hiding this comment.
"Fuses the tag-dispatched borrow and the read, no scratch." reads very AI-slopish, can just remove, instruction name is self documenting?
| src, | ||
| size, | ||
| } => { | ||
| // Fuses the tag-dispatched borrow and the write, no |
There was a problem hiding this comment.
Same here re comment: can just remove this (instruction name explains) and keep non-overlapping invariant only.
| }, | ||
| Instr::WriteField(_, _, _, val) => { | ||
| Instr::ReadFieldChain(_, path, _) => { | ||
| b.add_sized(REF_RW_BASE, REF_RW_PER_BYTE, self.chain_terminal_ty(path)?) |
There was a problem hiding this comment.
For all chaining ops, I think we better have a cost same as the original chain? Otherwise change to optimization pass is a breaking change and needs feature gating and kills the purpose of moving gas instrumentation to IR? I think we should instrument before optimizations (which iirc we do not do and run after instead)

Chained Field Access Fusion
Overview
This PR teaches the mono-move specializer to recognize chains of struct field borrows — patterns like
&s.a.b.cfollowed by a read or write — and collapse each chain into a single fused instruction. Where the interpreter previously executed one micro-op per field step (each computing an address, materializing a reference, and dispatching back through the run loop), a fused chain now executes as one micro-op that folds the entire path into a single precomputed offset.Main Changes
New fused IR instructions
The stackless IR gains a family of field chain instructions covering the useful combinations of root (a local, or an existing reference) and terminal action (borrow immutably, borrow mutably, read the value out, or write a value in). Each carries the full field path, so downstream phases see the whole access as one unit. Borrow chains deliberately preserve the immutable/mutable distinction, because a mutable borrow of a local creates a hidden-write channel that later optimization passes (coalescing, copy propagation) must be able to see.
The fusion pass
A new pass runs during destacking, before slot allocation. It scans each block for runs of single-use field borrows that feed directly into one another and ends each run at its terminal use. Key properties:
Enum variant-field access rework
Enum field access micro-ops were restructured into two tiers:
All offsets are precomputed as object-relative (the enum tag header is folded in at construction), so the interpreter does no offset arithmetic beyond a single add.
Runtime and gas
Testing
Note
Medium Risk
Touches VM interpreter memory access, enum tag dispatch semantics, and optimizer soundness around hidden writes through mut borrows; behavior is heavily regression-tested but errors would affect execution correctness.
Overview
Adds pre-slot-allocation fusion that turns depth-≥2 runs of single-use struct field borrows (local or ref roots, read/write/borrow terminals, optional gap before the terminal) into
*FieldChainstackless instructions, then lowers them with summed offsets via shared helpers. Gas meters chains as one access by terminal field size, not depth.Enum variant fields are split into uniform
HeapReadOffset/HeapWriteOffsetvs tag-dispatchedEnum*VariantFieldByTag(sharedvariant_field_locin the interpreter). Divergent read/write no longer use a 16-byte scratch fat pointer;variant_field_scratchframe reservation is removed.Runtime fix:
Move8uses unaligned 8-byte copies so misaligned size-8 aggregates are valid.Copy propagation / destack analysis use
mut_local_borrow_target(includingMutBorrowLocalFieldChain). New differential tests cover struct chains, enum divergent paths, abort order, and generics.Reviewed by Cursor Bugbot for commit bd474d4. Bugbot is set up for automated code reviews on this repo. Configure here.